Also recognize .h++ as a C++ header extension#9346
Also recognize .h++ as a C++ header extension#9346anshul-garg27 wants to merge 1 commit intowarpdotdev:masterfrom
Conversation
|
I'm starting a first review of this pull request. You can follow along in the session on Warp. I approved this pull request. No matching stakeholder was found for the changed files, so no human reviewers were requested. Comment I approved this pull request and requested human review from: @vorporeal, @alokedesai, @zachbai. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR maps .hpp, .hxx, and .h++ file extensions to the existing C++ language support and adds a unit test covering the new extensions.
Concerns
- No blocking correctness, security, error handling, or performance concerns found in the changed lines.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
There was a problem hiding this comment.
Overview
This PR maps .hpp, .hxx, and .h++ filename extensions to the existing C++ language support and adds a unit test covering those new header extensions.
Concerns
- No blocking correctness, security, error-handling, or performance concerns found in the changed lines.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
Heads-up — issue #9387 was filed earlier today for this exact bug, and #9388 covers a partly-overlapping set: If this lands first it will close #9387 too (linked via the new "Fixes #9387" trailer once added). Pushed an updated body with the issue link. |
|
Hi, I'd missed this one. Since #9388 merged, could you rebase on top of that with the unified set of extensions? I'll approve+merge once CI passes. Thanks! |
Rebased on top of warpdotdev#9388 (which added `hpp`, `hxx`, and the uppercase `H` form). This adds the rarer `h++` extension as well so the case list covers the full set of C++ header conventions, and extends the existing `cpp_header_extensions_resolve_to_cpp_language` test to include `header.h++`.
anshul-garg27
force-pushed
the
fix/cpp-header-extensions
branch
from
8155450 to
73e17fa
Compare
anshul-garg27
changed the title
|
I'm checking this implementation PR for association with a likely matching ready issue. Powered by Oz |
3 participants
Rebased on top of #9388 (which landed `hpp`, `hxx`, and the uppercase `H` form, per @bnavetta's request).
This keeps just the one delta from the original PR: adding `h++` to the match arm so the case list covers the full set of C++ header conventions, and extending the existing `cpp_header_extensions_resolve_to_cpp_language` test to include `header.h++`.
`h++` is the rarer of the C++ header conventions but it's part of the canonical set (used in some older codebases and called out in the GNU coding style guidance for C++).
Testing
~inwarp://action/new_tab?path=URLs #9277), but CI should cover it.Server API
No server changes.
Agent Mode
Not applicable.
Changelog Entries
`CHANGELOG-IMPROVEMENT`: Recognize `.h++` files as C++ headers (in addition to `.hpp`, `.hxx`, and `.H` from #9388).